I cannot compile the test target unless I add this import, which seems to come from a missing rake generate somewhere else.

The latest commit in trunk doesn't have this import and passed CI though, could it be from packages not being fully resolved / a need to build the app before tests / a need for a clean build?

I tried removing this line, building the app then ran POSCouponTests, both the app and tests ran. I've had frustrating times when it took a few tries rebuilding/cleaning though. If you get to build and run tests without this line, let's remove this.

Using a previous date for dateExpires while isExpired is comparing against Date() makes it not exactly fit the goal of the test case test_isExpired_when_current_date_then_returns_true, which is when the expiration date is the same as the current date.

A common approach is to mock the Date() in the code we're testing. e.g. from the existing POSCoupon.isExpired:

to be a function that takes in currentDate: Date = Date():

public func isExpired(currentDate: Date = Date()) -> Bool {
    guard let dateExpires else {
        return false
    }
    return dateExpires < currentDate
}

This way, we can pass any date values for the current date and expiration date.

Also, from the logic, wouldn't isExpired return false when the current date and dateExpires are the same? 🤔 Like it expires right on the expiration date, which sounds like the expected behavior.

This way, we can pass any date values for the current date and expiration date.

Yes, so my point with "As a 3rd option, we could change the details of POSCoupon, but I don't see a good reason to change the interface only because of tests." is:

If we would change the isExpired computed var to be a function that might expect a date to be passed in there should be a reason that is not testing only, right? Since we never pass any date in the implementation, nor is expected, my first gut check was that it shouldn't be a function neither accept a date as param, as this change to the implementation would be only for the sake of tests. So should we actually make the change?

I'm happy with making the change if you feel is worth it, just questioning in case this comes up again on a different place with a similar shape 🤔 otherwise might be better to just remove it.

👋 I hadn't seen this PR, but fixed this in a simpler way...

WDYT about just making it return dateExpires <= Date()? That makes it match the existing tests without changing any interfaces.

Thanks Josh! Let's go ahead with your PR, its more correct to treat them as expired as soon as the date is equal to expiration, fixing the flakiness as well. I'll mark this one as closed.

The latest commit in trunk doesn't have this import and passed CI though, could it be from packages not being fully resolved / a need to build the app before tests / a need for a clean build?

I tried removing this line, building the app then ran POSCouponTests, both the app and tests ran. I've had frustrating times when it took a few tries rebuilding/cleaning though. If you get to build and run tests without this line, let's remove this.